-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Another round removing warnings #24500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| /** stringOf formatted for use in a repl result. */ | ||
| def replStringOf(arg: Any, maxElements: Int): String = | ||
| stringOf(arg, maxElements) match { | ||
| case null => "null toString" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stringOf(null, maxElements) = "null", this is indeed not reachable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was for a non-null value for which toString returns null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, in this case, stringOf should return String | Null. I will remove that commit from this PR until we check what we will do in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the catch @som-snytt!
| def next(): R = | ||
| if(!hasNext) Iterator.empty.next() | ||
| else node match { | ||
| else node.nn match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the call to hasNext before makes sure that indeed node is not null. This was an exhaustivity warning (missing case null).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elidable was deprecated in #23913.
c45ea37 to
55db92a
Compare
#24500 was on auto-merge and it failed because the positions were changed. This PR fixes the positions while I also updated the branch requirements for a PR to include all the jobs (excluding some of the non-bootstrapped for now).
So far, we still have pattern matching warnings, init warnings, a bug in the deprecatedOverride logic that triggers warnings because of generated code and some CC warnings too.